Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

otlp: Add beta support to receive logs #6768

Merged
merged 7 commits into from
Dec 7, 2021

Conversation

marclop
Copy link
Contributor

@marclop marclop commented Dec 1, 2021

Motivation/summary

Adds support to receive OTLP logs on the GRPC receiver. This feature is
beta and should be used sparsely until the OTel spec declares it as
stable.

Currently, it's challenging to test this in the system tests since the
opentelemetry-go SDK doesn't expose any APIs for logs.

Checklist

How to test these changes

  1. docker-compose up -d
  2. make
  3. ./apm-server -E output.elasticsearch.username=admin -E output.elasticsearch.password=changeme -e
  4. Run the following go program: (Make sure that you're using go.opentelemetry.io/collector/[email protected]):
package main

import (
	"context"
	"time"

	"go.opentelemetry.io/collector/model/otlpgrpc"
	"go.opentelemetry.io/collector/model/pdata"
	semconv "go.opentelemetry.io/collector/model/semconv/v1.5.0"
	"google.golang.org/grpc"
)

func main() {
	conn, err := grpc.Dial("localhost:8200", grpc.WithInsecure(), grpc.WithBlock())
	if err != nil {
		panic(err)
	}
	defer conn.Close()

	client := otlpgrpc.NewLogsClient(conn)
	newLog := func(body interface{}) pdata.Logs {
		logs := pdata.NewLogs()
		resourceLogs := logs.ResourceLogs().AppendEmpty()
		logs.ResourceLogs().At(0).Resource().Attributes().InsertString(
			semconv.AttributeTelemetrySDKLanguage, "go",
		)
		instrumentationLogs := resourceLogs.InstrumentationLibraryLogs().AppendEmpty()
		otelLog := instrumentationLogs.Logs().AppendEmpty()
		otelLog.SetTraceID(pdata.NewTraceID([16]byte{1}))
		otelLog.SetSpanID(pdata.NewSpanID([8]byte{2}))
		otelLog.SetName("doOperation()")
		otelLog.SetSeverityNumber(pdata.SeverityNumberINFO)
		otelLog.SetSeverityText("Info")
		otelLog.SetTimestamp(pdata.NewTimestampFromTime(time.Now()))
		otelLog.Attributes().InsertString("key", "value")
		otelLog.Attributes().InsertDouble("key", 1234)

		switch b := body.(type) {
		case string:
			otelLog.Body().SetStringVal(b)
		case int:
			otelLog.Body().SetIntVal(int64(b))
		case float64:
			otelLog.Body().SetDoubleVal(float64(b))
		case bool:
			otelLog.Body().SetBoolVal(b)
		}
		return logs
	}

	_, err = client.Export(context.Background(), newLog("my log message"))
	if err != nil {
		panic(err)
	}
	_, err = client.Export(context.Background(), newLog(12334))
	if err != nil {
		panic(err)
	}
}
  1. Install the APM Integration
  2. Verify that the two log messages show up in the Observability > Logs UI.

image

Related issues

Closes #5491

@marclop marclop added enhancement v8.0.0 backport-8.0 Automated backport with mergify labels Dec 1, 2021
@apmmachine
Copy link
Contributor

apmmachine commented Dec 1, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-12-07T05:01:50.035+0000

  • Duration: 41 min 51 sec

  • Commit: 457ce9b

Test stats 🧪

Test Results
Failed 0
Passed 5616
Skipped 19
Total 5635

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /hey-apm : Run the hey-apm benchmark.

  • /package : Generate and publish the docker images.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@marclop marclop force-pushed the f/support-otel-logs branch from 110e612 to 017039d Compare December 2, 2021 02:55
Adds support to receive OTLP logs on the GRPC receiver. This feature is
_beta_ and should be used sparsely until the OTel spec declares it as
stable.

Currently, it's challenging to test this in the system tests since the
`opentelemetry-go` SDK doesn't expose any APIs for logs.

Signed-off-by: Marc Lopez Rubio <[email protected]>
@marclop marclop force-pushed the f/support-otel-logs branch 2 times, most recently from 6808b14 to 835558e Compare December 6, 2021 08:07
Signed-off-by: Marc Lopez Rubio <[email protected]>
@marclop marclop force-pushed the f/support-otel-logs branch from 835558e to 25aec1f Compare December 6, 2021 08:11
@marclop marclop marked this pull request as ready for review December 6, 2021 10:30
@marclop marclop requested a review from a team December 6, 2021 10:31
@marclop marclop changed the title otlp: Add experimental support to receive logs otlp: Add beta support to receive logs Dec 6, 2021
changelogs/head.asciidoc Outdated Show resolved Hide resolved
processor/otel/logs.go Show resolved Hide resolved
if event.Span == nil {
event.Span = &model.Span{}
}
event.Span.ID = spanID.HexString()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this also refer to the transaction.ID. Is there any information available that would allow a similar check as https://github.com/elastic/apm-server/blob/master/processor/otel/traces.go#L211-L226?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can know that here, but happy to be told otherwise. We're going to need to do something like elastic/apm#413, renaming transaction.id to span.id.

I think it might be best to set span.id even though it might not be correct, i.e. because it's a transaction. At least then users can search for a transaction with the span.id value, and we'll be aligned for a future where transaction.id doesn't exist.

@felixbarny WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM. We could also add a span.id to transactions and look for ways how to phase out transaction.id going forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, thanks

processor/otel/logs_test.go Outdated Show resolved Hide resolved
processor/otel/logs.go Outdated Show resolved Hide resolved
if event.Span == nil {
event.Span = &model.Span{}
}
event.Span.ID = spanID.HexString()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can know that here, but happy to be told otherwise. We're going to need to do something like elastic/apm#413, renaming transaction.id to span.id.

I think it might be best to set span.id even though it might not be correct, i.e. because it's a transaction. At least then users can search for a transaction with the span.id value, and we'll be aligned for a future where transaction.id doesn't exist.

@felixbarny WDYT?

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you 🎉

Can you please add the fields to app_logs, unless there's a reason not to?

model/log.go Show resolved Hide resolved
model/event.go Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Dec 7, 2021

This pull request is now in conflicts. Could you fix it @marclop? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b f/support-otel-logs upstream/f/support-otel-logs
git merge upstream/master
git push upstream f/support-otel-logs

@marclop marclop enabled auto-merge (squash) December 7, 2021 04:03
@marclop marclop merged commit 5e5623e into elastic:master Dec 7, 2021
@marclop marclop deleted the f/support-otel-logs branch December 7, 2021 05:42
mergify bot pushed a commit that referenced this pull request Dec 7, 2021
Adds support to receive OTLP logs on the GRPC receiver. This feature is
_beta_ and should be used sparsely until the OTel spec declares it as
stable.

Signed-off-by: Marc Lopez Rubio <[email protected]>
(cherry picked from commit 5e5623e)

# Conflicts:
#	changelogs/head.asciidoc
marclop pushed a commit that referenced this pull request Dec 7, 2021
Adds support to receive OTLP logs on the GRPC receiver. This feature is
_beta_ and should be used sparsely until the OTel spec declares it as
stable.

Signed-off-by: Marc Lopez Rubio <[email protected]>
(cherry picked from commit 5e5623e)
@cristianmad
Copy link

This is awesome! Is there a beta build we could try?

Thank you!

@marclop
Copy link
Contributor Author

marclop commented Dec 8, 2021

@cristianmad We'll publish a release candidate (8.0.0-rc1) at the end of December, that would be the safest way to beta test this feature before 8.0.0 is officially released.

@stuartnelson3
Copy link
Contributor

confirmed with 2d1c270

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenTelemetry] add support for receiving logs
7 participants